-
Notifications
You must be signed in to change notification settings - Fork 58
Bug fix: allow local builds without requiring client directory #243
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, I can't get this to work as written.
If I change the value of clients_built
to false
then it does indeed not need the clients to build successfully (yay!) but if then try to run ./build/init-topics-and-clients.sh ../valkey-doc/topics ../valkey-doc/clients
I get sed: 1: "content/clients/_index.md": command c expects \ followed by text
and it doesn't flip the front matter boolean.
content/clients/_index.md
Outdated
@@ -2,6 +2,7 @@ | |||
title = "Client Libraries" | |||
template = "client-list.html" | |||
[extra] | |||
clients_built = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be set to false
initially?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, sorry about that — must’ve been left over from an earlier iteration when I was testing it
@stockholmux did you run it on macOS? I’ve updated the |
Yep. I was testing on macOS. Any other flavours of The other thought that comes to mind now is if we're going to have constant problems with people accidentally committing |
Okay! I played around with this and I think I found a way around The problem here stems from If you put |
Signed-off-by: Lior Sventitzky <[email protected]>
So, I enclosed all of the content of this page with a check at the beginning - trying to use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Description
Currently building the zola site locally is not possible when the clients directory is not built, as the
zola serve
command fails.This is fixed by introducing an additional boolean variable
clients_built
in the client's page frontmatter, which is set to false by default, but updates to be true when running theinit_topics_and_clients.sh
script successfully. The page loads only if this var is set to true, otherwise a "Clients page not found" message is displayed, similar to the behavior in the commands and topics documentation.Issues Resolved
Check List
--signoff
By submitting this pull request, I confirm that my contribution is made under the terms of the BSD-3-Clause License.